test: verify frame counts and geometry against the pipeline, not the model - #79
Merged
Merged
Conversation
…model FrameMap was covered only as arithmetic — output_count, inverse, total_radius — which proves the model is self-consistent and says nothing about whether the plugins agree with it. The Rust suite cannot close that gap by design: run_job generates and inspects the .vpy and never runs vspipe | ffmpeg. integration_frame_mapping_test.dart encodes and counts for every pass that changes the count: double-rate QTGMC 31 -> 62, single-rate as the control 31 -> 31, IVTC cycle 5 90 -> 72, and FlowFPS 25 -> 23.976 75 -> 71. Expected values are derived by hand from the FrameMap definitions rather than read back from the model, so a change to the model cannot quietly redefine "correct". The retime case pins why FlowFPS was chosen over BlockFPS: n*num/den exactly, where BlockFPS gives floor((n-1)*r)+1 and every retimed job's progress total would be a frame out. It also checks the preview. --frame N is a SOURCE index, so under double-rate the preview of source frame S must be output frame 2S. Measured 0.00 mean abs diff at the right frame against 8-30 at its neighbours. The assertion is relative — the correct frame must be the closest of five — because an absolute tolerance loose enough to absorb the encoder and the colour conversion would also be loose enough to pass on a preview that is off by a frame. integration_upscale_resize_test.dart already asserted dimensions for upscaling and seven downscale kernels, but crop had no dimension assertion anywhere: the existing crop tests ran a crop and checked the job succeeded, which passes just as happily on a crop that did nothing. Adds asymmetric crop size, crop composed with resize, and crop pixels rather than crop size — a size assertion cannot tell a correct crop from one that swaps left with right, so the offset test compares against the pipeline's own uncropped output cropped in Dart, with the swapped offsets as negative controls (1.06 correct against 9.65 and 15.97). Plus preview/render agreement for crop, resize and both together, which also asserts the size, since a preview at the wrong resolution surfaces as a buffer-length mismatch in meanAbsDiff rather than as a pixel difference. Two traps found writing this, both recorded in CLAUDE.md. totalFrames is the POST-TRIM count. Trimming is decoder-side and pipe_source builds a fixed-length clip from that number, so declaring the full source length alongside a trim makes pipe_source pad by repeating the last real frame on EOF. This test was written that way first and reported 150 frames for a 31-frame trim, which reads exactly like FPSDivisor being ignored. The app and main.rs both already do it correctly. select=eq(n,N) on a source is not the pipeline's frame N. interlaced_test.avi declares 79 frames and decodes 75 — it carries null frames, and the decoder's default CFR mode expands them onto the 25 fps grid, so the first seven decoded frames come out 0,0,0,0,1,1,2. select counts decoded frames while the pipeline counts timeline positions. Correct behaviour, but it makes ffmpeg-side frame indexing useless as a reference, hence comparing pipeline output against pipeline output. WorkerHarness.frameRgb24 is the shared helper for that. It decodes from the start rather than seeking, because an input seek lands on the nearest keyframe and would silently compare the wrong frame.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FrameMapwas covered only as arithmetic —output_count,inverse,total_radius. That proves the model is self-consistent and says nothing about whether the plugins agree with it. The Rust suite can't close the gap by design:run_jobgenerates and inspects the.vpyand never runsvspipe | ffmpeg.Frame counts, from real encodes
integration_frame_mapping_test.dart(new):Expected values are derived by hand from the FrameMap definitions rather than read back from the model, so a change to the model can't quietly redefine "correct". The retime case pins why FlowFPS was chosen over BlockFPS —
n*num/denexactly, where BlockFPS givesfloor((n-1)*r)+1and every retimed job's progress total would be a frame out.Preview sync
--frame Nturns out to be a source index (the worker logs "Preview: source frame N"), so under double-rate the preview of source frame S must be output frame 2S. Measured 0.00 mean abs diff at the right frame against 8–30 at its neighbours — bit-exact.The assertion is deliberately relative: the correct frame must be the closest of five candidates. An absolute tolerance loose enough to absorb the encoder and the colour conversion would also be loose enough to pass on a preview that's off by a frame.
Scaling and cropping
integration_upscale_resize_test.dartalready asserted dimensions for upscaling and seven downscale kernels, but crop had no dimension assertion anywhere — the existing crop tests ran a crop and checked the job succeeded, which passes just as happily on a crop that did nothing.Adds asymmetric crop size, crop composed with resize, and crop pixels rather than crop size: a size assertion can't tell a correct crop from one that swaps left with right, since both give the requested dimensions. The offset test compares against the pipeline's own uncropped output cropped in Dart, with the swapped offsets as negative controls (1.06 correct against 9.65 and 15.97).
Plus preview/render agreement for crop, resize and both together — 0.00 at all three. These also assert the size, because a preview at the wrong resolution surfaces as a buffer-length mismatch in
meanAbsDiffrather than as a pixel difference.Two traps found writing this
totalFramesis the POST-TRIM count. Trimming is decoder-side andpipe_sourcebuilds a fixed-length clip from that number, so declaring the full source length alongside a trim makes it pad by repeating the last real frame on EOF. This test was written that way first and reported 150 frames for a 31-frame trim — which reads exactly like FPSDivisor being ignored. I checked before claiming a bug: the app andmain.rsboth already do it correctly.select=eq(n,N)on a source is not the pipeline's frame N.interlaced_test.avideclares 79 frames and decodes 75 — it carries null frames, and the decoder's default CFR mode expands them onto the 25 fps grid, so the first seven decoded frames come out0,0,0,0,1,1,2.selectcounts decoded frames while the pipeline counts timeline positions. Correct behaviour, but it makes ffmpeg-side frame indexing useless as a reference — hence comparing pipeline output against pipeline output.Both recorded in
CLAUDE.md.WorkerHarness.frameRgb24is the shared helper, decoding from the start rather than seeking (an input seek lands on the nearest keyframe and would silently compare the wrong frame).Note on what CI will actually verify here
Both files are
@Tags(['heavy']), soci-test.ymlexcludes them — the push gate will confirm nothing else regressed, but the new tests themselves run innightly.yml. Locally: 23 heavy tests pass across the two files, and the push gate is at 820.